…e hardening)

Addresses the 8 findings from the independent code review (see PR apache#4366
comments). 49/49 contrib tests still pass on BOTH Spark 4.1 + Delta 4.1.0
and Spark 3.5 + Delta 3.3.2 after these changes.

Critical:

  1. native/shuffle/src/spark_unsafe/unsafe_object.rs: replace
     `from_utf8_unchecked` with `from_utf8_lossy` returning `Cow<'_, str>`.
     The previous version constructed a `&str` from arbitrary bytes
     (Spark's binary-cast-to-string case, e.g. Delta's Z-Order
     `interleave_bits(...).cast(StringType)`) -- the Rust reference defines
     that as UB even when the bytes only get copied downstream, because
     downstream Arrow ops internally use `str::from_utf8_unchecked` on the
     StringArray buffer and would propagate the UB. `from_utf8_lossy` is
     well-defined: zero-cost borrow for valid UTF-8, allocates a String
     with U+FFFD replacements for invalid bytes (only fires on the
     binary-cast case, which Spark never displays as text anyway). All
     call sites pass to `StringBuilder::append_value` which takes
     `AsRef<str>`; `Cow<str>`'s `AsRef<str>` impl makes them work
     transparently. No call-site changes.

  2. DeltaIntegration.scala: narrow the `case _: Exception => None`
     swallow in `transformV1IfDelta` to ONLY catch true reflection
     binding failures (`NoSuchMethodException`/`NoSuchFieldException`/
     `IllegalAccessException`) and invocation errors
     (`IllegalAccessException`/`IllegalArgumentException`). An
     `InvocationTargetException` -- the contrib's transform actually
     threw -- now log-warnings and declines instead of silently falling
     back to vanilla. Without this, kernel-rs IO errors, CCE on a Delta
     version bump, NPE in the CM-id translator etc. would silently
     decline and the user would never know. Same narrowing applied to
     `scanHandler` and `DeltaPlanDataInjector` lookup (operators.scala).

Should-fix:

  3. CometExecRDD.compute: don't set InputFileBlockHolder when a
     partition has multiple files. Previous code took
     `partition.filePaths.head` always, which would silently report the
     first file's path for every row when a contrib accidentally batched
     multiple files in one partition. (Tried `require(length == 1)`
     first; that's too strict because partitioned reads legitimately have
     multi-file partitions but don't query `input_file_name()`. Skipping
     the hook on multi-file partitions preserves correctness for
     `input_file_name()` callers -- which MUST one-task-per-partition
     anyway -- without false-positive failing legitimate partitioned
     reads.)

  4. engine.rs: LRU-bound the engine cache at MAX_CACHE_ENTRIES=32. The
     cache key included `DeltaStorageConfig` which contains
     `aws_session_token`; long-running drivers with rotating STS/IRSA
     credentials would grow one entry per rotation and LEAK one
     `TokioBackgroundExecutor` thread per stale entry. With LRU eviction,
     `Arc<DeltaEngine>` drops on eviction, `DefaultEngine` drops its
     `TokioBackgroundExecutor`, the OS thread joins, thread count
     stabilizes. Test `get_or_create_engine_evicts_lru_when_full`
     verifies the bound + eviction order.

Nits:

  5. planner.rs: error message for the "DeltaScan in default build" case
     now mentions BOTH `-Pcontrib-delta` (Maven) and `--features
     contrib-delta` (Cargo) -- previously mentioned only the Cargo flag.

  6. dev/verify-contrib-delta-gate.sh: also assert the contrib-enabled
     libcomet has >0 Delta-related external symbols. Without this, a
     future Rust toolchain change that mangles symbol names differently
     would silently turn the default-build symbol check into a no-op
     while still passing -- the gate would lie about being enforced.
     Asserting both "default has 0" AND "contrib has >0" catches grep
     pattern drift.

Build infrastructure:

  7. pom.xml + spark/pom.xml: move `<delta.version>` default to the
     parent POM's top-level properties. Per-Spark-profile `delta.version`
     overrides cleanly (spark-3.5 -> 3.3.2, spark-4.1 -> 4.1.0), and
     spotless-style invocations without a Spark profile still resolve
     the property. The previous arrangement (default in `contrib-delta`
     profile) had Spark-profile overrides silently lose to the
     contrib-delta default because of POM profile-document-order property
     precedence.

  8. Make `PlanDataInjector` and `DeltaIntegration` extend
     `org.apache.spark.internal.Logging` so the new `logWarning` calls
     compile.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…gnore conflict

Introduce a small extension contract so out-of-tree Comet contrib leaf scans
(Delta, and future Hudi/etc.) can participate in native planning without core
holding a compile-time reference to them -- mirroring the Iceberg-precedent of
keeping the data-source-specific code at the edge.

What this adds:
- `trait CometScanWithPlanData` (`sourceKey` / `commonData` / `perPartitionData`,
  plus optional `dynamicPruningFilters` / `withDynamicPruningFilters` for scans
  whose DPP filters live in a @transient field). `CometNativeScanExec` now mixes
  it in.
- `CometNativeExec.foreachUntilCometInput` matches `case _: CometLeafExec` (a
  strict superset of the previous fixed scan enumeration -- all built-in leaf
  scans already extend `CometLeafExec`), so any leaf Comet exec is recognised as
  an input boundary.
- `PlanDataInjector.findAllPlanData` collects per-partition planning data via the
  trait instead of a hardcoded `CometNativeScanExec` match.
- `PlanDataInjector`'s registry gains one reflective `DeltaPlanDataInjector$`
  slot, appended only when a contrib bundled it (`-Pcontrib-delta`). Default
  builds get a `ClassNotFoundException` -> `None` and an unchanged injectors list,
  so there is zero contrib surface at runtime.
- `CometPlanAdaptiveDynamicPruningFilters` rewrites AQE DPP filters in place for
  trait scans whose filters can't survive `makeCopy` (apache#3510).

Inert by construction: with no contrib on the classpath this is behavior-
preserving (the leaf match is a superset; the trait match catches the same
`CometNativeScanExec`; the reflective slot resolves to nothing).

Tests: `CometScanWithPlanDataSuite` (trait-contract defaults + reflective-slot
graceful absence). Verified `CometJoinSuite` (native scan fusion / DPP) stays
green.

First unit of the Delta-contrib PR split (tracking: apache#4366).

Introduce a small extension contract so out-of-tree Comet contrib leaf scans
(Delta, and future Hudi/etc.) can participate in native planning without core
holding a compile-time reference to them -- mirroring the Iceberg-precedent of
keeping the data-source-specific code at the edge.

What this adds:
- `trait CometScanWithPlanData` (`sourceKey` / `commonData` / `perPartitionData`,
  plus optional `dynamicPruningFilters` / `withDynamicPruningFilters` for scans
  whose DPP filters live in a @transient field). `CometNativeScanExec` now mixes
  it in.
- `CometNativeExec.foreachUntilCometInput` matches `case _: CometLeafExec` (a
  strict superset of the previous fixed scan enumeration -- all built-in leaf
  scans already extend `CometLeafExec`), so any leaf Comet exec is recognised as
  an input boundary.
- `PlanDataInjector.findAllPlanData` collects per-partition planning data via the
  trait instead of a hardcoded `CometNativeScanExec` match.
- `PlanDataInjector`'s registry gains one reflective `DeltaPlanDataInjector$`
  slot, appended only when a contrib bundled it (`-Pcontrib-delta`). Default
  builds get a `ClassNotFoundException` -> `None` and an unchanged injectors list,
  so there is zero contrib surface at runtime.
- `CometPlanAdaptiveDynamicPruningFilters` rewrites AQE DPP filters in place for
  trait scans whose filters can't survive `makeCopy` (apache#3510).

Inert by construction: with no contrib on the classpath this is behavior-
preserving (the leaf match is a superset; the trait match catches the same
`CometNativeScanExec`; the reflective slot resolves to nothing).

Tests: `CometScanWithPlanDataSuite` (trait-contract defaults + reflective-slot
graceful absence). Verified `CometJoinSuite` (native scan fusion / DPP) stays
green.

First unit of the Delta-contrib PR split (tracking: apache#4366).

Introduce a small extension contract so out-of-tree Comet contrib leaf scans
(Delta, and future Hudi/etc.) can participate in native planning without core
holding a compile-time reference to them -- mirroring the Iceberg-precedent of
keeping the data-source-specific code at the edge.

What this adds:
- `trait CometScanWithPlanData` (`sourceKey` / `commonData` / `perPartitionData`,
  plus optional `dynamicPruningFilters` / `withDynamicPruningFilters` for scans
  whose DPP filters live in a @transient field). `CometNativeScanExec` now mixes
  it in.
- `CometNativeExec.foreachUntilCometInput` matches `case _: CometLeafExec` (a
  strict superset of the previous fixed scan enumeration -- all built-in leaf
  scans already extend `CometLeafExec`), so any leaf Comet exec is recognised as
  an input boundary.
- `PlanDataInjector.findAllPlanData` collects per-partition planning data via the
  trait instead of a hardcoded `CometNativeScanExec` match.
- `PlanDataInjector`'s registry gains one reflective `DeltaPlanDataInjector$`
  slot, appended only when a contrib bundled it (`-Pcontrib-delta`). Default
  builds get a `ClassNotFoundException` -> `None` and an unchanged injectors list,
  so there is zero contrib surface at runtime.
- `CometPlanAdaptiveDynamicPruningFilters` rewrites AQE DPP filters in place for
  trait scans whose filters can't survive `makeCopy` (apache#3510).

Inert by construction: with no contrib on the classpath this is behavior-
preserving (the leaf match is a superset; the trait match catches the same
`CometNativeScanExec`; the reflective slot resolves to nothing).

Tests: `CometScanWithPlanDataSuite` (trait-contract defaults + reflective-slot
graceful absence). Verified `CometJoinSuite` (native scan fusion / DPP) stays
green.

First unit of the Delta-contrib PR split (tracking: apache#4366).

Introduce a small extension contract so out-of-tree Comet contrib leaf scans
(Delta, and future Hudi/etc.) can participate in native planning without core
holding a compile-time reference to them -- mirroring the Iceberg-precedent of
keeping the data-source-specific code at the edge.

What this adds:
- `trait CometScanWithPlanData` (`sourceKey` / `commonData` / `perPartitionData`,
  plus optional `dynamicPruningFilters` / `withDynamicPruningFilters` for scans
  whose DPP filters live in a @transient field). `CometNativeScanExec` now mixes
  it in.
- `CometNativeExec.foreachUntilCometInput` matches `case _: CometLeafExec` (a
  strict superset of the previous fixed scan enumeration -- all built-in leaf
  scans already extend `CometLeafExec`), so any leaf Comet exec is recognised as
  an input boundary.
- `PlanDataInjector.findAllPlanData` collects per-partition planning data via the
  trait instead of a hardcoded `CometNativeScanExec` match.
- `PlanDataInjector`'s registry gains one reflective `DeltaPlanDataInjector$`
  slot, appended only when a contrib bundled it (`-Pcontrib-delta`). Default
  builds get a `ClassNotFoundException` -> `None` and an unchanged injectors list,
  so there is zero contrib surface at runtime.
- `CometPlanAdaptiveDynamicPruningFilters` rewrites AQE DPP filters in place for
  trait scans whose filters can't survive `makeCopy` (apache#3510).

Inert by construction: with no contrib on the classpath this is behavior-
preserving (the leaf match is a superset; the trait match catches the same
`CometNativeScanExec`; the reflective slot resolves to nothing).

Tests: `CometScanWithPlanDataSuite` (trait-contract defaults + reflective-slot
graceful absence). Verified `CometJoinSuite` (native scan fusion / DPP) stays
green.

First unit of the Delta-contrib PR split (tracking: apache#4366).

…b split, part 2]

Part 2 of the Delta Lake contrib PR breakup (tracking: apache#4366). Establishes the
`contrib-delta` build gate and the inert wiring that lets a gated build compile
end to end, while the DEFAULT build stays byte-for-byte unchanged (zero Delta
surface). No real Delta read logic yet -- that lands in later parts; here a Delta
read that reaches native returns a clean "not implemented" error and falls back
to vanilla Spark.

Build gate:
- Maven `contrib-delta` profile (spark/pom.xml) with per-Spark `delta.version`
  (3.5->3.3.2, 4.0->4.0.0, 4.1->4.1.0) and an add-source of contrib/delta/src.
  Default `delta.version` floor in pom.xml. The default spark.version stays 4.1.2
  (the delta-spark 4.1.1 pin is a separate, deferred decision).
- Cargo `contrib-delta` feature on core (optional path dep on comet-contrib-delta);
  `native/Cargo.toml` excludes ../contrib from the workspace.
- `dev/verify-contrib-delta-gate.sh` proves default cargo/mvn/dylib carry zero
  Delta surface and the gated build pulls the right deps; wired into a minimal
  `delta_build_gate.yml` CI job (the full suite/regression workflows land later).
  Hardened the script against a `set -o pipefail` + `grep -q` SIGPIPE misfire
  (early grep exit -> echo SIGPIPE -> false guard failure) via here-strings.

Inert wiring:
- Proto: `Delta*` messages + `delta_scan = 118` (117 is BroadcastNestedLoopJoin).
- Native dispatch: `OpStruct::DeltaScan` arm with a not-compiled-in error on
  default builds and a feature-gated `delta_scan` shim that calls the contrib;
  exhaustive-match arms in operator_registry/jni_api; `convert_spark_types_to_
  arrow_schema` promoted to pub(crate).
- Stub contrib crate (contrib/delta/native): `plan_delta_scan` returns
  `DataFusionError::NotImplemented` -- just enough to satisfy the core shim's
  contract so `--features contrib-delta` links.
- JVM bridge `DeltaIntegration` (reflective, all lookups return None until the
  contrib classes exist), the CometExecRule Delta-marker hook (CDF hook deferred
  to a later part), the CometScanRule Delta delegation + metadata-col reorder,
  and the leaf `DeltaConf`.

Verification: default + gated native build, clippy both feature states, gate
script, gated + default JVM compile, spotless/scalastyle, cargo fmt -- all green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

… + JNI) [Delta contrib split, part 3a]

Part 3a of the Delta Lake contrib PR breakup (tracking: apache#4366). Replaces the build-gate
stub crate's deps with the real driver-side modules and the delta-kernel-rs dependency,
while the executor-side read path stays deferred (the `planner` stub still returns
NotImplemented until part 3b adds `kernel_scan`/`dv_reader` + the real planner).

Driver side (open table, replay log, push predicates, return a DeltaScanTaskList over JNI):
- `error.rs`  - DeltaError / DeltaResult.
- `engine.rs` - delta-kernel engine + object_store config (S3/Azure/GCS/local) for log replay.
- `predicate.rs` - Catalyst -> kernel predicate translation for file skipping.
- `scan.rs`   - log replay -> DeltaFileEntry/DeltaScanPlan; scan-task assembly.
- `jni.rs`    - `Native_planDeltaScan` / `Native_planDeltaReadSchemas` JNI entry points
  (the JVM `Native.scala` that calls them lands in part 4b).
- `lib.rs`    - declares the driver modules + keeps the `planner` stub; drops the
  `dv_reader`/`kernel_scan` module decls (part 3b).
- `Cargo.toml` - only the deps the driver set uses (delta_kernel, object_store, arrow,
  jni, prost, serde_json, url, thiserror, log + jni-bridge); executor deps arrive in 3b.

The driver set is self-contained (`jni -> scan -> engine -> error`, `predicate` standalone);
nothing references the deferred modules. Core is untouched -- the dispatch shim still calls
`planner::plan_delta_scan` (the stub) so a Delta read falls back to vanilla Spark until 3b.
The gate-verify cargo-tree assertion is re-tightened to require `delta_kernel` (now real).

Verification: gated native build, 54 in-crate unit tests (cargo test), default native build
unchanged, clippy (both feature states), gate-verify script, cargo fmt -- all green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…s) [Delta contrib split, part 3b]

Part 3b of the Delta Lake contrib PR breakup (tracking: apache#4366). Completes the contrib
native crate: the executor-side read path replaces the build-gate stub planner, so a
`-Pcontrib-delta` build now does end-to-end native Delta reads (given a scan task, read
through delta-kernel-rs, apply the transform + deletion vectors).

- `planner.rs` - replaces the stub: assembles the per-task `DataSourceExec` (parquet scan +
  partition values + DV filter), wired to the core dispatch shim's `plan_delta_scan` call.
- `kernel_scan.rs` - the kernel read bridge (`planner` <-> `kernel_scan` are mutually
  dependent and ship together): schema resolution, column-mapping, row-tracking, transform.
- `dv_reader.rs` - Delta deletion-vector decode (inline + on-disk roaring bitmaps), surfaced
  as a DataFusion filter; missing-DV-file maps to SparkError::FileNotFound for parity.
- `lib.rs` - re-adds the `dv_reader`/`kernel_scan` module decls and the
  `DeltaScan`/`DeltaScanCommon` proto re-exports trimmed in 3a.
- `Cargo.toml` - re-adds the executor deps (parquet, roaring, datafusion-datasource, futures,
  chrono*, comet-common, tokio dev-dep) deferred from 3a.

Core is untouched -- the dispatch shim is unchanged; it now reaches the real planner instead
of the stub. The native crate is now equivalent to the integration branch (modulo the crate
version, kept at 0.18.0, and a clarified planner doc-link). Default builds still carry zero
Delta surface.

Verification: gated native build, 89 in-crate unit tests (54 driver + 35 executor), default
native build unchanged, clippy (both feature states), gate-verify script (contrib libcomet
+13 MB), cargo fmt -- all green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…lit, part 4a]

Part 4a of the Delta Lake contrib PR breakup (tracking: apache#4366). The JVM claim/decline
layer: `DeltaScanRule` recognises a V1 Delta scan and plants a `CometDeltaScanMarker`
wrapping the original scan. This "activates" the reflective path A.2 already wired
(`DeltaIntegration` looks up `DeltaScanRule$`), but stays INERT end to end: with no serde
yet (`CometDeltaNativeScan`, part 4b), `CometExecRule`'s `scanHandler` lookup returns None,
so the marker is left in the plan and executes as a vanilla Delta fallback. Net behavior on
this build: Delta reads still run on vanilla Spark.

- `DeltaScanRule.scala` - claim/decline rule (declines input_file_name, encryption, etc.;
  plants the marker otherwise).
- `CometDeltaScanMarker.scala` - leaf exec node wrapping the original `FileSourceScanExec`;
  `doExecute` delegates to it (the vanilla fallback). FQN matches A.2's `DeltaIntegration.
  MarkerClass` string.
- `DeltaScanMetadata.scala` - planning info carried on the marker; now also home to the
  `ScanImpl` constant (moved off the not-yet-present serde so the rule can name it).
- `DeltaReflection.scala` - reflective Delta accessors (CDF members inert until part 5).
- `RowTrackingAugmentedFileIndex.scala` - row-tracking file index used by the rule.
- Tests: `CometDeltaTestBase` (trimmed of the serde/exec-dependent native-read helpers, which
  move to 4b; added marker-claim helpers) + a new `CometDeltaMarkerSuite` asserting the marker
  is planted on a plain read, the fallback is result-correct, and a declined projection plants
  no marker.
- `dev/ci/check-suites.py` - exempt contrib test suites (they run under the dedicated Delta
  workflow, not the default pr_build matrix).

No core / A.2 edits needed -- the reflective wiring already reaches this rule + marker.
Required edit per the split plan: `CometDeltaNativeScan.ScanImpl` -> `DeltaScanMetadata.ScanImpl`
(4b re-points the serde at it).

Verification: gated JVM test-compile, `CometDeltaMarkerSuite` 3/3 (marker planted is red on the
A.2 build / green here), check-suites, spotless + scalastyle, gate-verify (default build still
0 Delta symbols, only the DeltaIntegration bridge class) -- all green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

apache#4535 main, review round 2 clean

…); apache#4366 sync deferred to end-of-cycle

…flaky re-run) + apache#4366 carveout links

…on table + 411-line pre-refactor tech-ref → docs pointer)